Skip to content

Shape PyMuPDF4LLM OCR config for upstream: reactive failure, declarative config#2

Open
veget-able wants to merge 2 commits into
pymupdf:pymupdf4llm-ocr-layoutfrom
veget-able:pymupdf4llm-ocr-shape
Open

Shape PyMuPDF4LLM OCR config for upstream: reactive failure, declarative config#2
veget-able wants to merge 2 commits into
pymupdf:pymupdf4llm-ocr-layoutfrom
veget-able:pymupdf4llm-ocr-shape

Conversation

@veget-able

Copy link
Copy Markdown

Note

Companion to #1 - independent, no ordering requirement. Both PRs auto-merge cleanly in the provider file (the test file has a trivial both-append conflict; happy to rebase whichever lands second).

Summary

Shapes the PyMuPDF4LLM provider's OCR handling for eventual upstream (run-llama) submission. One commit, 3 files.

  • Drop the proactive pymupdf.get_tessdata() probe. It was the only proactive engine probe in the tree, cost ~two subprocess spawns per request (~70 ms floor, up to ~350 ms/page observed), and its tesseract --list-langs output breaks UTF-8 subprocess readers on non-UTF-8 Windows locales. Tesseract now fails reactively at use time (ImportError/missing tessdata -> ProviderConfigError), matching how the rest of the tree treats OCR engines.
  • Keep pipeline config declarative. ocr_backend stays a plain string; the engine callable is resolved internally at call time (module map) and passed straight to to_markdown(ocr_function=...) - it never enters the options dict that gets serialized into run metadata.
  • Config keys deliberately mirror the pymupdf4llm API (use_ocr, force_ocr, ocr_dpi, ocr_language are 1:1 to_markdown kwargs) - mirroring the wrapped library beats mirroring other providers' vocabulary; noted in a code comment.
  • docs/pipelines.md rows for the five pymupdf4llm pipelines.
  • Tests: 15 pass (14 added), covering backend resolution, declarative options, and reactive failure.

Note on residual stderr noise

Removing the probe silences the provider-level noise, but the default pipeline still triggers the same get_tessdata() subprocess inside pymupdf4llm itself (library default use_ocr=True + ocr_function=None). That residual is library-internal (no ParseBench frame in the stack), out of scope here, and unchanged from the base branch. pymupdf4llm_markdown_no_ocr runs with completely clean stderr.

…lve backend internally

Clean the PyMuPDF4LLM provider's OCR surface so it can go to public upstream.

Remove the proactive `pymupdf.get_tessdata()` probe (was in `_markdown_options`
for `ocr_backend="tesseract"`). It was the only proactive engine probe in the
tree and it spawns a `tesseract --list-langs` subprocess: ~350 ms/page as
measured in review (~70 ms/call floor locally, two subprocess spawns), and under
PYTHONUTF8=1 on a Korean (cp949) console its output fails UTF-8 decode inside
subprocess reader threads, printing uncatchable "Exception in thread /
UnicodeDecodeError" noise. Tesseract now fails reactively like the rest of
upstream: an unavailable engine surfaces as ProviderConfigError only when that
backend is actually requested (at import in `_resolve_ocr_function`), not eagerly
for every request. The old probe carried no fallback semantics (it only raised),
so nothing else was needed.

Stop injecting the `ocr_function` callable into the options dict. `ocr_backend`
stays a plain string config key (it is a ParseBench-level selector, not a
to_markdown kwarg); the engine module/function is resolved internally from a
module-level map at call time and passed to `to_markdown(ocr_function=...)` as a
direct local kwarg, immediately before the call. `_markdown_options()` now
returns only declarative, serializable library kwargs -- important because the
runner json-dumps `pipeline.config` into run metadata; keeping callables out of
any options/config dict keeps that serialization clean.

Mirror the pymupdf4llm library API for config-key names: `use_ocr`, `force_ocr`,
`ocr_dpi`, `ocr_language` map 1:1 to the fork build's `to_markdown` kwargs and
are forwarded verbatim, so keep those names (documented in a code comment). This
consciously answers the review suggestion to rename `ocr_dpi`->`dpi` / drop
`force_ocr`: mirroring the library the provider wraps beats mirroring another
provider's ad-hoc naming.

docs/pipelines.md: add the five pipelines this branch introduces
(pymupdf4llm_markdown + _tesseract/_rapidocr/_no_ocr/_150dpi). html_tables is
intentionally left for the table-output PR.

Tests: extend test_pymupdf4llm.py for the removed probe (get_tessdata is not
called while building options), the declarative options dict (no ocr_function /
ocr_backend keys, no callables), internal backend resolution via the module map,
and reactive failure of an unavailable backend.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-up (Hashir): dropping the proactive get_tessdata() probe
also dropped the only explicit availability check for the tesseract
backend. pymupdf4llm's tesseract_api imports cleanly without Tesseract
(it warns once and exec_ocr becomes a per-page no-op), so the
ImportError guard in _resolve_ocr_function never fires for it and an
explicitly requested tesseract run would silently proceed without OCR
- quietly mis-scoring the benchmark. (rapidocr is not affected: its
module import raises when rapidocr_onnxruntime is missing.)

Restore the fail-fast at the agreed layer: _resolve_ocr_function reads
the backend module's import-time TESSDATA marker (no extra subprocess
probe, no callables or side effects in the declarative options) and
raises ProviderConfigError when it is None. Config stays pure and
serializable; the failure is still reactive - it only surfaces when the
tesseract backend is actually requested.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_d5691145-f68e-4159-a913-572249137769
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant